Implement event ownership #423
Conversation
r-c-n
force-pushed
the
event-ownership
branch
7 times, most recently
from
b0c6cb0 to
33d0a80
Compare
|
Thanks for the proposed changes. The PR looks pretty useful.
I don't think any additional subscription field is required here. If the user omits |
Thanks for reviewing.
Not with the changes in this PR, that's a fundamental difference and it's worth it to discuss what's the most reasonable approach. Currently, there's no server-side filtering of events whatsoever, so all the filtering is done by clients in the way you explained. That means that every client will receive all events if they didn't specify any subscription filter, if I understood correctly. In previous conversations, the consensus was that the server should provide the necessary filtering to avoid flooding the clients and (IMO) to provide a basis for a more secure event dispatching. For example, in the future we might want to implement private events that will be sent only to certain authorized users. The key feature in this PR is event/subscription ownership. When a client creates a subscription, it's automatically set as the owner of it. Then, events sent through the |
r-c-n
force-pushed
the
event-ownership
branch
from
33d0a80 to
8202e90
Compare
r-c-n
force-pushed
the
event-ownership
branch
2 times, most recently
from
b462201 to
7704982
Compare
|
Btw, pylint is failing because: But that's not true AFAICT? |
I see |
Simplify the PublishEvent model and define it so that only the "data"
field is mandatory. "type" and "source" are optional and the API fills
them by default if undefined. "attributes" is an optional dict to
specify additional event attributes.
An additional attribute ("owner") will be automatically added by the API
when necessary.
Signed-off-by: Ricardo Cañuelo <[email protected]>
This field will associate a Subscription to the user that requested it and will be used later to do server-side filtering of events based on the event owner. Signed-off-by: Ricardo Cañuelo <[email protected]>
r-c-n
force-pushed
the
event-ownership
branch
3 times, most recently
from
f84e47d to
9b0b26a
Compare
Keep track of the Subscription object associated to each subscription id so that it can be retrieved later on "listen" operations. This will allow the API to check for the Subscription "user" and filter out unwanted messages if necessary. Signed-off-by: Ricardo Cañuelo <[email protected]>
Let the caller specify optional attributes or no attributes at all. Fill in 'type' and 'source' with default values if undefined. Signed-off-by: Ricardo Cañuelo <[email protected]>
If the event specifies an owner, send it only to the subscriptions created by that user. Signed-off-by: Ricardo Cañuelo <[email protected]>
Signed-off-by: Ricardo Cañuelo <[email protected]>
…ttributes Sync the publish endpoint to the current implementation of pubsub.publish_cloudevent(). Add a 'owner' attribute in the event to save the username of the authenticated request user. Signed-off-by: Ricardo Cañuelo <[email protected]>
Signed-off-by: Ricardo Cañuelo <[email protected]>
r-c-n
force-pushed
the
event-ownership
branch
2 times, most recently
from
da20c80 to
f14ef50
Compare
Let a user unsubscribe only from his/her own subscriptions. Prior to this change, anyone could unsubscribe anybody else from whatever active subscription. Signed-off-by: Ricardo Cañuelo <[email protected]>
Signed-off-by: Ricardo Cañuelo <[email protected]>
r-c-n
force-pushed
the
event-ownership
branch
from
f14ef50 to
15659b9
Compare
Signed-off-by: Ricardo Cañuelo <[email protected]>
r-c-n
force-pushed
the
event-ownership
branch
from
15659b9 to
fd6d216
Compare
|
Tested OK on staging. |
Introduction
This PR features a first basic step towards proper event filtering and handling. User ownership data is added to events and subscriptions, and the ownership is checked by default in "listen" and "unsubscribe" operations.
Functional changes
Changes to PublishEvent and Subscription
The
Subscriptionmodel now includes auserfield to store the username associated to the subscription (ie. the "owner"). NOTE: This assumes that usernames are unique.The
PublishEventmodel was simplified and the processing of events through thepublishendpoint now includes an implicit definition of the event owner and it allows overriding the default "type" and "source" attributes (if that's ever needed, if not we can remove this) and adding other extra attributes freely. From a users perspective, the only mandatory field in the request isdata(dict). Any other extra attributes can be defined inattributes(dict).Example payload for a
publishrequest with a custom extra attribute:'{"data" : {"my_event_id" : 1}, "attributes" : {"level" : "critical"}}'. The owner attribute is added automatically by the server.Listening for events now receives only those published by the same user and "anonymous" events
By default, a request to the
listenendpoint will only receive events owned by the same user that's listening. Anonymous events (ie. events that don't define anowner, such as keepalive events) are still received by all subscribers.(See "Disabling user-checking on event listening" below for alternatives).
Users can unsubscribe their own subscriptions only
Check user ownership of subscriptions to prevent a user from removing a subscription that another user created.
Ownership of node creation events
All node creation events published by the
nodeandnodesendpoints now have the authenticated request user as the owner, so only this user will receive these events.Open questions
Better integration of the User handling functionalities into this
This PR is very basic and it doesn't make full use of the user management capabilities that are currently implemented. We may want to make a better use of these capabilities, for example, to do permission and ownership checks based on user groups.
Disabling user-checking on event listening
Even if we want to keep and check the ownership of a subscription (to let only the owner unsubscribe, for instance), there'll probably be use cases where a privileged user might want to implement a monitor or a debugger that can listen to any arbitrary event regardless of its owner. This can be implemented by using an additional Subscription attribute to define its type ("regular-user subscription", "monitor-mode subscription", etc)
Features based on user capabilities
Certain (future) features could be made available only to certain users or user groups, such as admins.
Server-side event filtering
Currently, events are filtered in the client side by helper functions. The same thing can be done in the server side, moving the processing to the server and hiding unwanted data from ever reaching the clients. Would this be useful or desirable?